Thanks for reporting.

I took a brief look, and it seems to get more complicated, since false already has conflicting tests:

    context 'with false' do
      let(:params) { {id: false} }

      it { expect(expanded.to_s).to eq 'http://test.host/resource/' }
    end

I tend to change this to include false too... 🤔

Thanks for checking! I saw this "conflicting" test too, but wouldn't consider it a conflict necessarily: The test you cited is regarding the path parameters, not regarding query parameters. Theoretically, one (path) could behave differently than the other (query). Of course, I understand that this might be even more misleading...

Also, I was unsure about the inclusion of nil as a query parameter. I followed the path syntax (where the value is ommitted as well), but added the key. Of course, this decision can be discussed, too. I think, it is rather a design-decision how the interface should look like.

Unfortunately, Restify does not parse and fill URI templates, but explicitly delegates to Addressable::Template#example only. Restify cannot differ between path and non-path placeholders, but only between URI templates variables and extra query params.

Addressable::Template also hard-skips nils: https://github.com/sporkmonger/addressable/blob/main/lib/addressable/template.rb#L759.

Alright. Then, I'll refactor the nil test, so that the key is omitted as well.

Thanks! I do like ?abc though, but I really do not want to compile the template on my own 🙈 ...

No problem -- I liked ?abc, too, but I agree that compiling the template is not worth the effort. 👍

?abc only works, when the URI template does not include a {abc} placeholder. Then they are added as query values directly:

let(:pattern)  { '/resource/{id}{?q*}' }

context 'with additional nil parameters' do
  let(:params) { {id: '5', abc: nil} }

  it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc' }
end

Right, this part is also covered in the specs already:

I would still suggest adding the explicit test for the placeholder (especially since it is removed completely), just to have this behavior documented in the code: https://github.com/MrSerth/restify/blob/8318c08d343336e87f28a50a25d0092688320536/spec/restify/relation_spec.rb#L62-L67

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.04%. Comparing base (fe526fe) to head (af29bb3).
Report is 5 commits behind head on main.

@@            Coverage Diff             @@
##             main      #68      +/-   ##
==========================================
+ Coverage   95.88%   96.04%   +0.15%     
==========================================
  Files          21       21              
  Lines         681      683       +2     
==========================================
+ Hits          653      656       +3     
+ Misses         28       27       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.